Revert "fix: construct User object from flat AuthResponse fields in auth store"#79
Conversation
There was a problem hiding this comment.
Pull request overview
This PR reverts a previous frontend change that constructed a User from a flat AuthResponse, switching the frontend back to expecting a nested user object in auth responses.
Changes:
- Removed login/register unit tests that validated behavior with a flat
AuthResponse. - Changed
AuthResponseTypeScript type from flat user fields to{ token, user }. - Updated the auth store to directly use
response.userinlogin()/register()and persist it to localStorage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| frontend/src/views/tests/auth.store.test.ts | Removes login/register coverage related to flat auth responses. |
| frontend/src/types/index.ts | Changes AuthResponse shape to include a nested user. |
| frontend/src/store/auth.ts | Updates login/register to read and persist response.user (no mapping). |
Comments suppressed due to low confidence (1)
frontend/src/views/tests/auth.store.test.ts:102
- The login/register unit tests that asserted auth state + localStorage persistence were removed. With this change set (and especially given the auth response shape is being modified), there’s no longer test coverage ensuring
login()/register()populateuser,token, and storage correctly. Consider reintroducing tests for the current expected response contract (flat vs nested) so regressions in auth flows are caught.
describe('useAuthStore – mocking the SAML user directly (unit-test style)', () => {
beforeEach(() => {
setActivePinia(createPinia())
localStorage.clear()
vi.resetAllMocks()
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| username: string | ||
| email: string | ||
| role: UserRole | ||
| user: User |
There was a problem hiding this comment.
AuthResponse was changed to { token, user }, but the backend currently returns a flat auth response (token, userId, username, email, role) (see backend/.../AuthDto.kt:30-36). With this type shape, frontend code will compile but response.user will be undefined at runtime unless the backend contract is also changed. Either revert this TS type back to the flat shape or update the backend DTO/controller to return user: { ... } (including createdAt) consistently.
| user: User | |
| userId: number | |
| username: string | |
| email: string | |
| role: UserRole |
| async function login(credentials: LoginRequest): Promise<void> { | ||
| const response = await authApi.login(credentials) | ||
| token.value = response.token | ||
| const loginUser = userFromAuthResponse(response) | ||
| user.value = loginUser | ||
| user.value = response.user | ||
| localStorage.setItem('token', response.token) | ||
| localStorage.setItem('user', JSON.stringify(loginUser)) | ||
| localStorage.setItem('user', JSON.stringify(response.user)) | ||
| } | ||
|
|
||
| async function register(payload: RegisterRequest & { role?: UserRole }): Promise<void> { | ||
| const response = await authApi.register(payload) | ||
| token.value = response.token | ||
| const regUser = userFromAuthResponse(response) | ||
| user.value = regUser | ||
| user.value = response.user | ||
| localStorage.setItem('token', response.token) | ||
| localStorage.setItem('user', JSON.stringify(regUser)) | ||
| localStorage.setItem('user', JSON.stringify(response.user)) |
There was a problem hiding this comment.
login() and register() now read response.user and persist it. Given the backend auth endpoints currently return flat fields (no user object), this will set user.value to undefined, store invalid JSON, and leave isAuthenticated false after a successful login/register. Consider mapping the backend response into a User (e.g., reintroduce a userFromAuthResponse transform or do the mapping in @/api/auth) or change the backend response to include user.
No description provided.